fix(sandbox_web): guard Playwright fallback redirects#723
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| profile_id="unknown_challenge", | ||
| success_selectors=["article#c"], | ||
| force_executor="playwright_real_chrome", | ||
| scope_check=lambda candidate: "example.com" in candidate, |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24331aac78
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # otherwise follow that redirect outside the curl SSRF guard. | ||
| if any((a.error or "").startswith("ssrf_redirect_blocked:") for a in trace): | ||
| skip_browser = True | ||
| stop_reason = stop_reason or "ssrf_redirect_blocked" |
There was a problem hiding this comment.
When any curl attempt records ssrf_redirect_blocked: after the grid later sets stop_reason to "exhausted" or "budget", this assignment keeps that existing value, so _give_up() still reports normal exhaustion and advertises playwright_mcp as an untried escalation. In the exact scenario this guard is trying to suppress, the caller can be told to retry the attacker-controlled redirect in an agent browser; treat the SSRF redirect block as the terminal stop reason instead of only filling it when empty.
Useful? React with 👍 / 👎.
| att.error = f"ssrf_blocked:{reason}" | ||
| att.url = final_url | ||
| return att, "" | ||
| if scope_check is not None and not scope_check(final_url): |
There was a problem hiding this comment.
This RoE check runs only after _run_node_template() has already allowed Chromium to navigate and return the final page. With scope_check set, an in-scope URL that redirects to an out-of-scope public host will still be fetched by the browser before being marked blocked here, unlike the curl path's pre-hop gate; the scope decision needs to be enforced inside the Playwright route before route.continue().
Useful? React with 👍 / 👎.
| # template runs without xvfb; set INSANE_HEADLESS=0 (with xvfb present) | ||
| # to run headful, which evades headless-detecting WAFs (Akamai/DataDome). | ||
| "headless": os.environ.get("INSANE_HEADLESS", "1") not in ("0", "false", "no"), | ||
| "allowPrivate": False, |
There was a problem hiding this comment.
In environments that intentionally set INSANE_ALLOW_PRIVATE=1, the curl transport and the Python final-URL check use safety.allow_private_default(), but the browser template is always invoked with allowPrivate: false. That makes the new route guard abort loopback/RFC1918 requests before the Python-side opt-in can permit them, regressing local/private engagements that require Playwright fallback.
Useful? React with 👍 / 👎.
2 participants
Motivation
Description
scope_checkparameter intorun_playwright_fallbackand the fetch chain so per-hop RoE gating can be enforced for fallback runs.executor.run_playwright_fallbacknow classifies thefinalUrlviasafety.classify_urland rejects/block-returns when the final URL is private/blocked or out of RoE scope instead of trusting the browser output.fetch_chainprevents invoking the browser fallback if the curl transport trace already recorded anssrf_redirect_blocked:error to avoid retrying an attacker-controlled URL with the browser tier.ipaddr.js) and aborts requests that resolve to private/loopback/link-local/reserved or fail DNS resolution;ipaddr.jswas added to the templates'package.json.Testing
DECEPTICON_SKIP_BOOT=1 pytest packages/decepticon/tests/unit/sandbox_web/test_executor.py packages/decepticon/tests/unit/sandbox_web/test_fetch_chain.py packages/decepticon/tests/unit/sandbox_web/test_safety.pyand observed all tests pass (20 passed, 1 warning).node --checkon both Playwright templates andpython -m py_compileon modified Python modules, both succeeded with no syntax errors.ipaddr.jsand thatgit diff --checkproduced no whitespace or trailing-eol issues.Codex Task